Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: failed valuer convert return unexpected true #214

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

laushunyu
Copy link
Contributor

@laushunyu laushunyu commented Jul 1, 2024

when the field of struct implement driver.Valuer and cannot convert to dest type directly, copier.set() will return a unexpected (true, nil)

testcode:

package main

import (
	"database/sql/driver"
	"encoding/json"
	"fmt"

	"github.com/jinzhu/copier"
)

type IntArray []int

func (a IntArray) Value() (driver.Value, error) {
	return json.Marshal(a)
}

type Int int

type From struct {
	Data IntArray
}

type To struct {
	Data []Int
}

func main() {
	var (
		from = From{
			Data: IntArray{1, 2, 3},
		}
		to To
	)
	if err := copier.Copy(&to, from); err != nil {
		panic(err)
	}

	fmt.Printf("%#v", to)
}

output:

main.To{Data:[]main.Int(nil)}

fixed:

main.To{Data:[]main.Int{1, 2, 3}}

@laushunyu
Copy link
Contributor Author

laushunyu commented Jul 1, 2024

@jinzhu review needed pls

@jinzhu 带带弟弟

@jinzhu
Copy link
Owner

jinzhu commented Jul 9, 2024

Hi @laushunyu

Can you add some tests for this?

@laushunyu
Copy link
Contributor Author

laushunyu commented Jul 10, 2024

Hi @laushunyu

Can you add some tests for this?

Hi!

testcase added.

in current master:

=== RUN   TestValuerConv
    copier_converter_test.go:272: copier failed from copier_test.From{Data:copier_test.IntArray{1, 2, 3}} to copier_test.To{Data:[]copier_test.Int(nil)}
--- FAIL: TestValuerConv (0.00s)

FAIL

after fixed:

=== RUN   TestValuerConv
copier_test.To{Data:[]copier_test.Int{1, 2, 3}}--- PASS: TestValuerConv (0.00s)
PASS

@jinzhu jinzhu merged commit 659270e into jinzhu:master Jul 18, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants